Thanks for the contribution, but I don't think we can take this as-is — both the threat model and the chosen fix have problems.

1. No trust boundary is being crossed. metadata_filter and filepath_globpattern come from the same client query schema (RetrieveQuerySchema / FilterSchema) at the same trust level. metadata_filter is, by design, an arbitrary JMESPath expression supplied by the caller. So a client that can set filepath_globpattern can already set metadata_filter to anything it wants — there is nothing to "short-circuit past". The "multi-tenant isolation enforced via a silent metadata_filter" scenario isn't how the Document Store API actually works.

2. Not code injection / CWE-94. JMESPath here is a read-only filter language (jmespath.search over in-memory metadata in vector_store.py / _knn_lsh.py), not eval. There's no code execution; worst case is filter manipulation, which (see point 1) the caller already fully controls.

3. The allowlist breaks legitimate glob patterns — functional regression. globmatch is backed by fnmatch per path level (_knn_lsh.py), which supports character classes and other glob syntax. The regex ^[a-zA-Z0-9_\-\*\?\.\/\\ ]+$ rejects valid inputs:

'data[0-9]/*.pdf'  -> valid glob, rejected
'*.{pdf,txt}'      -> rejected
'file[!a].pdf'     -> rejected

These are normal user queries that would now raise ValueError mid-pipeline.

4. Inconsistent even on its own terms. If literal-breakout were the concern, the stronger vector is metadata_filter (full JMESPath, also string-interpolated at line 37-40), which this PR leaves untouched. It hardens the weaker of the two identical channels.

5. Title mismatch. The title mentions a "ReDoS" fix, but nothing in the diff addresses ReDoS.

Minor: raising ValueError inside a pw.udf errors the row/pipeline rather than rejecting the query gracefully, and the test reaches into .__wrapped__.

If we did want to harden the string interpolation for robustness, the right direction would be escaping the single quote in the value (as is already partially done for metadata_filter) or building the expression via parameterization — not an allowlist that strips valid glob syntax. Given the above, I'm going to close this. Thanks again for looking into it.

Thanks for the contribution, but I don't think we can take this as-is — both the threat model and the chosen fix have problems.

1. No trust boundary is being crossed. metadata_filter and filepath_globpattern come from the same client query schema (RetrieveQuerySchema / FilterSchema) at the same trust level. metadata_filter is, by design, an arbitrary JMESPath expression supplied by the caller. So a client that can set filepath_globpattern can already set metadata_filter to anything it wants — there is nothing to "short-circuit past". The "multi-tenant isolation enforced via a silent metadata_filter" scenario isn't how the Document Store API actually works.

2. Not code injection / CWE-94. JMESPath here is a read-only filter language (jmespath.search over in-memory metadata in vector_store.py / _knn_lsh.py), not eval. There's no code execution; worst case is filter manipulation, which (see point 1) the caller already fully controls.

3. The allowlist breaks legitimate glob patterns — functional regression. globmatch is backed by fnmatch per path level (_knn_lsh.py), which supports character classes and other glob syntax. The regex ^[a-zA-Z0-9_\-\*\?\.\/\\ ]+$ rejects valid inputs:

'data[0-9]/*.pdf'  -> valid glob, rejected
'*.{pdf,txt}'      -> rejected
'file[!a].pdf'     -> rejected

These are normal user queries that would now raise ValueError mid-pipeline.

4. Inconsistent even on its own terms. If literal-breakout were the concern, the stronger vector is metadata_filter (full JMESPath, also string-interpolated at line 37-40), which this PR leaves untouched. It hardens the weaker of the two identical channels.

5. Title mismatch. The title mentions a "ReDoS" fix, but nothing in the diff addresses ReDoS.

Minor: raising ValueError inside a pw.udf errors the row/pipeline rather than rejecting the query gracefully, and the test reaches into .__wrapped__.

If we did want to harden the string interpolation for robustness, the right direction would be escaping the single quote in the value (as is already partially done for metadata_filter) or building the expression via parameterization — not an allowlist that strips valid glob syntax. Given the above, I'm going to close this. Thanks again for looking into it.

Thanks for the incredibly detailed breakdown, Sergey!

That makes total sense—I had incorrectly assumed metadata_filter was a backend-enforced boundary for multi-tenancy, but since both inputs share the exact same client trust boundary, you are completely right that this isn't an isolation bypass. Great catch on the fnmatch character classes as well; my regex allowlist was definitely way too strict and would have broken valid queries.

I really appreciate you taking the time to explain the architectural context so thoroughly. I completely agree with closing this one, and I've just updated the ReDoS fix over on #250 with the performance test and changelog updates you requested!